feat(webapp): dashboard agent — chat, reports, investigate - #4418
feat(webapp): dashboard agent — chat, reports, investigate#4418kathiekiwi wants to merge 520 commits into
Conversation
🦋 Changeset detectedLatest commit: 2aacd26 The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThe PR adds dashboard-agent contracts, database persistence for messages, investigations, and watches, modular agent tools, evaluation controls, and conversation compaction. It adds delegated-token environment scoping, bounded request handling, queue metrics, report APIs, health telemetry states, report caching, and structured report rendering. The webapp gains dashboard-agent components, investigation and report cards, URI resolution, CSP image-origin handling, theme-aware branding, route authorization changes, maintenance sweeps, and related tests and documentation. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
internal-packages/dashboard-agent/src/tool-schemas.ts (1)
547-559: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAlign the capability instructions with the new mutating tools.
The prompt now exposes
schedule_watch,create_alert, anddelete_alert, but it still describes the toolset as read-only and later says the agent cannot change anything. This contradiction can make the agent refuse supported watch/alert actions or incorrectly direct users to the dashboard. Update the blanket capability text to distinguish read-only data tools from these explicitly authorized mutations.
🧹 Nitpick comments (1)
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx (1)
351-368: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winMemoize the transcript-derived winner map.
Line 357 creates a new
Mapon every render, and Line 368 passes it tomemoized turns; activity-only updates therefore rerender the entire transcript and rescan all parts. Memoizestrippedand its winners frommessages.Proposed change
- const stripped = messages.map(stripStepParts); - - const investigationWinners = winningInvestigationOccurrences(stripped); + const { stripped, investigationWinners } = useMemo(() => { + const stripped = messages.map(stripStepParts); + return { + stripped, + investigationWinners: winningInvestigationOccurrences(stripped), + }; + }, [messages]);As per coding guidelines,
useMemois appropriate for expensive derived data and stable references required by dependency arrays.Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ae8860fd-cece-44f8-b032-c969b4234488
📒 Files selected for processing (2)
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxinternal-packages/dashboard-agent/src/tool-schemas.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (33)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
- GitHub Check: sdk-compat / Node.js 22.23 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Node.js 24.18 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Bun Runtime
- GitHub Check: sdk-compat / Node.js 26.4 (warp-ubuntu-latest-x64-4x)
- GitHub Check: sdk-compat / Node.js 20.20 (warp-ubuntu-latest-x64-4x)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-ubuntu-latest-x64-4x - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-ubuntu-latest-x64-4x - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - npm)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: sdk-compat / Cloudflare Workers
- GitHub Check: sdk-compat / Deno Runtime
- GitHub Check: runops-guard / runops-guard
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: code-quality / code-quality
- GitHub Check: 🛡️ E2E Auth Tests (full)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamicimport(); use dynamic imports only for unresolvable circular dependencies, genuine performance code splitting, or conditional runtime loading.
Import Trigger.dev tasks from@trigger.dev/sdk; never use@trigger.dev/sdk/v3or deprecatedclient.defineJob.
Add agentcrumbs while writing code using approved namespaces; mark lines with//@Crumbsor blocks with `// `#region` `@crumbs, and strip them before merging.
Files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxinternal-packages/dashboard-agent/src/tool-schemas.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxinternal-packages/dashboard-agent/src/tool-schemas.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathDo not reintroduce the removed v1 execution path;
RunEngineVersion.V1branches may only reject or finalize gracefully so v3 clients receive a clean 4xx, never a 5xx.
Files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For apps, use
typecheckfor verification and never usebuildas the correctness check.
Files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
internal-packages/dashboard-agent/src/tool-schemas.ts
internal-packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For internal packages, use
typecheckfor verification and never usebuildas the correctness check.
Files:
internal-packages/dashboard-agent/src/tool-schemas.ts
🧠 Learnings (18)
📚 Learning: 2026-02-11T16:37:32.429Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3019
File: apps/webapp/app/components/primitives/charts/Card.tsx:26-30
Timestamp: 2026-02-11T16:37:32.429Z
Learning: In projects using react-grid-layout, avoid relying on drag-handle class to imply draggability. Ensure drag-handle elements only affect dragging when the parent grid item is configured draggable in the layout; conditionally apply cursor styles based on the draggable prop. This improves correctness and accessibility.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
📚 Learning: 2026-07-28T21:57:20.061Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 4411
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.settings.team/route.tsx:818-843
Timestamp: 2026-07-28T21:57:20.061Z
Learning: When using Radix UI `DialogClose` with `asChild` (e.g., Trigger.dev dashboard components), note that it injects `type="button"` into its child via `Slot`. If the child is a local `Button` that forwards its `type` prop to the native `<button>`, then placing it inside a `<form>` will *not* submit unless you explicitly set `type="submit"` (or otherwise override the injected type / wire up submission behavior). Review form actions to ensure the intended submit vs non-submit behavior is preserved.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxinternal-packages/dashboard-agent/src/tool-schemas.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxinternal-packages/dashboard-agent/src/tool-schemas.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxinternal-packages/dashboard-agent/src/tool-schemas.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxinternal-packages/dashboard-agent/src/tool-schemas.ts
📚 Learning: 2026-06-13T19:53:13.759Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3937
File: packages/trigger-sdk/skills/realtime-and-frontend/SKILL.md:258-260
Timestamp: 2026-06-13T19:53:13.759Z
Learning: When reviewing code that uses `trigger.dev/react-hooks`’s `useRealtimeRun`, preserve the call signature where the first argument is the full realtime handle object (not `handle.id`). This is intentional to maintain type-safety and is consistent with the official docs; do not suggest changing the first argument from the handle object to `handle.id`.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxinternal-packages/dashboard-agent/src/tool-schemas.ts
📚 Learning: 2026-06-17T17:13:49.929Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3948
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions.$bulkActionParam/route.tsx:48-62
Timestamp: 2026-06-17T17:13:49.929Z
Learning: In triggerdotdev/trigger.dev, within `dashboardLoader`/`dashboardAction` (or similar context resolver code) whenever you resolve an organization ID from an organization slug for RBAC/enterprise authorization scope, always read from the primary Prisma client (`prisma`), not `$replica`. Using `$replica` can hit replica-lag and cause the RBAC lookup/authorization to run without the correct org scope (bypassing intended role enforcement). Implement the slug→org lookup with `prisma.organization.findFirst(...)` (or equivalent primary-client query) and add an inline comment documenting why the primary client is required (replica lag could lead to unscoped RBAC checks).
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxinternal-packages/dashboard-agent/src/tool-schemas.ts
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxinternal-packages/dashboard-agent/src/tool-schemas.ts
📚 Learning: 2026-04-16T14:21:15.229Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/components/logs/LogsTaskFilter.tsx:135-163
Timestamp: 2026-04-16T14:21:15.229Z
Learning: When rendering lists of task registry items in apps/webapp (e.g., <SelectItem /> rows) and using `key={item.slug}`, do not flag it as potentially non-unique. In trigger.dev’s `TaskIdentifier` table, the DB constraint `@unique([runtimeEnvironmentId, slug])` guarantees `slug` is unique within a given runtime environment, so `item.slug` is safe as the React key as long as the list is derived from that registry/constraint (and not from a legacy query that could produce duplicate slugs).
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
📚 Learning: 2026-05-08T21:00:20.973Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3538
File: apps/webapp/app/components/primitives/Resizable.tsx:60-78
Timestamp: 2026-05-08T21:00:20.973Z
Learning: In the triggerdotdev/trigger.dev codebase, treat Zod as a boundary validation tool (API handlers, request/response validation, and storage/DB read/write validation), not as inline render-time validation inside React components/primitive UI code. For render-time guards, prefer small manual type-narrowing checks (e.g., a short predicate like ~10–20 lines) over importing Zod into UI primitives, to avoid per-render schema-parse overhead and unnecessary abstraction. Use the manual guard approach unless you truly need schema validation at a boundary; only then introduce Zod.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
📚 Learning: 2026-06-25T18:21:55.847Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/invite-resend.tsx:0-0
Timestamp: 2026-06-25T18:21:55.847Z
Learning: In the triggerdotdev/trigger.dev Zod 4 migration, avoid importing from the root package `conform-to/zod` in webapp code. It can resolve to the Zod 3 build and may crash at module load under Zod 4. When reviewing TypeScript/TSX files in `apps/webapp`, prefer importing from the Zod 4 subpath `conform-to/zod/v4` for Zod 4-compatible schemas/types.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.
Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.
Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
📚 Learning: 2026-06-25T18:21:51.905Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/invite-revoke.tsx:0-0
Timestamp: 2026-06-25T18:21:51.905Z
Learning: During the Zod v4 migration in the triggerdotdev/trigger.dev webapp, ensure any imports from `conform-to/zod` use the Zod-4 subpath: `conform-to/zod/v4` (e.g., `import { parseWithZod } from "conform-to/zod/v4"`). Do not import from the package root `conform-to/zod`, because it is the Zod 3 implementation and may load Zod-3-only symbols (e.g., `ZodBranded`, `ZodEffects`), which can throw at module load (notably with `zod4.4.3`). This should be enforced across `apps/webapp/**/*` where helpers like `parseWithZod` and `conformZodMessage` are used.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
📚 Learning: 2026-07-03T17:10:21.498Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4148
File: apps/webapp/app/models/orgMember.server.ts:149-168
Timestamp: 2026-07-03T17:10:21.498Z
Learning: In triggerdotdev/trigger.dev, `User.email` (Prisma schema: `internal-packages/database/prisma/schema.prisma`) currently does NOT use `citext` and does NOT have a `lower(email)` functional unique index. Therefore, do not introduce Prisma queries like `where: { email: { equals: <value>, mode: "insensitive" } }` (or any case-insensitive lookup) against `User.email`, because it can force sequential scans of the `users` table under load. During review, ensure email is normalized (e.g., lowercased/trimmed) before both writes and subsequent lookups, and if true case-insensitive behavior/uniqueness is required, implement it via a separate app-wide migration (e.g., switch to `citext` and/or add a functional unique index with backfill) rather than bolting it onto individual feature PRs.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
📚 Learning: 2026-06-25T18:21:54.729Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/confirm-basic-details.tsx:0-0
Timestamp: 2026-06-25T18:21:54.729Z
Learning: For Remix + TypeScript files that use Conform v1 (conform-to/react) and its getInputProps helper, when you intend to suppress the helper-provided default value for non-checkbox/non-radio inputs (e.g., hidden inputs managed via an explicit value prop), use the Conform v1 option key `value: false`. Do not recommend `defaultValue: false` here, because `defaultValue` is not a valid option key for these input types in Conform v1 typings.
Applied to files:
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
internal-packages/dashboard-agent/src/tool-schemas.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.
Applied to files:
internal-packages/dashboard-agent/src/tool-schemas.ts
🔇 Additional comments (1)
apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx (1)
99-146: LGTM!Also applies to: 245-287
- waiting-run diagnosis: 'unknown' with concurrency evidence in hand no longer claims the evidence is missing; an elapsed delay says 'not yet enqueued' instead of hiding behind time-from-creation - queue metrics route: drop the double decode that 500ed on names with a literal percent sign - evidence schema: kind must match the URI's own kind - seed-queue-metrics: default-binding imports like the other seeders
Observability mapAs of 19/100 over 416 measured of 432 entry points (base 18, up 1) What this PR changed
FIX FIRST
AUDIT 3 of 50 sensitive mutations record an actor. 47 without one. What the score is made ofThe score and findings here are report-only and never gate the merge. Separately, a required test suite keeps this tool's symbol and route lists in sync with the code they name, and can fail a pull request that renames or removes a symbol they reference, or that adds the first route with a segment they anticipate. Each failure names the list to edit. The rules and their reasons: internal-packages/observability-map/README.md. |
The webapp outgrew the default heap on CI runners — both E2E workflows died in webapp:build with allocation failures.
The dot's count and the toast's list both run on every poll of a closed panel, and both drove off a full scan of the watches table: the tenant predicates sat on the joined chat, so nothing narrowed the watch side first. Adds a partial index on (organization_id, user_id, resolution time desc) over delivered wakes only, and asserts the same org + user on the watch row so the planner can use it. That pair is redundant, not a new tenancy rule — a watch snapshots its chat's owner at creation and the create path rejects a chat from another org. On 200k watches: the count goes 20.5ms -> 0.5ms and the recent list 21ms -> 0.02ms, with the parallel seq scan replaced by an index scan that also absorbs the recency bound.
…in lockstep A background tab has no dot the user can see and nowhere to put a toast, so it stops asking; becoming visible triggers one immediate catch-up and restarts the cadence from there. Each delay carries fresh jitter, so many open tabs stop hitting the same second.
A report is ~9 ClickHouse queries, and the callers that dominate its volume are periodic rather than interactive: every watch tick in an environment asks for the same health verdict, so one sweep recomputed it once per watch. Caches the interpreted view model per (report, environment, period) for 90s — under the tick cadence, well over a burst. In-process and keyed by environment id, so nothing crosses a tenant. The single-flight map stays: it is what holds the first callers of a cold key to one load, and a rejected load is still never cached.
A terminal watch is read by nothing once its wake has landed — the chip is gone, dedup only looks at active rows, and the outcome's facts live in the chat transcript from then on. So the row was pure accumulation. The sweep now drops terminal rows whose last event was over 7 days ago, in one guarded statement bounded to 500 per run. Guards: terminal status only, delivery already settled (a row that still owes a wake is never taken), and the age measured from the newest timestamp on the row so a late delivery restarts the clock. Watches only — chats, messages and investigations are untouched.
The panel reloaded its chat list twice per turn — once on settle, then again on a 5s timer — purely because the generated name landed after the first reload and the list would otherwise sit on "New chat". The name is now written before the client settles instead. Generation starts in onTurnStart, so it runs alongside the model answering, and is awaited in onBeforeTurnComplete — the last hook before the turn-complete chunk closes the stream. onTurnComplete cannot do this: it fires after that chunk, which is exactly why the write used to miss. Costs nothing in practice (the cheap title model finishes long before the answer does) and a failure only loses the generated name. The delayed reload and its timer are gone, and a test pins the ordering — it fails if the await is dropped.
Follows the title write moving to onBeforeTurnComplete: the gate reads as a magic number without the note that a later turn cannot re-title anyway, because setChatTitleIfDefault only writes over the default name.
The catch around the turn body tolerated a JSON.parse failure, but it also swallowed a rejected mint and forwarded the turn with no credential.
The environment lookup, repo lookup and token mint now all run before the chat row is created, so a 404 or a mint failure can't orphan a chat in the history.
A token signed for a branch child listed nothing: the scope filter and the base-environments-only filter could never hold together. Unscoped callers still see base environments only.
identityOnly waives the contextless refusal, which is only sound for reads. The action options intersected the loader options, so the type allowed it.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts (1)
258-288: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPrevent orphaned chats when session startup fails.
createChatpersists the chat beforestartDashboardAgentHeadStartorstartDashboardAgentSessioncompletes. If either call rejects, the catch block returns HTTP 500 and leaves a chat with no persisted message or usable session.Make creation and startup atomic from the user view. Compensate for a failed startup by deleting the new chat, or defer persistence until the startup succeeds. Handle public-token mint failure separately so a live remote session is not deleted blindly. This regresses the stated no-orphan-chat behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 618b4c02-6b28-47f9-8e9e-94a4150c5333
📒 Files selected for processing (13)
.changeset/mint-token-cap-help.mdapps/webapp/app/presenters/v3/reports/health/health-data.tsapps/webapp/app/routes/api.v1.projects.$projectRef.environments.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.in.$.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.tsapps/webapp/app/services/routeBuilders/apiBuilder.server.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/dashboardAgentRoutes.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/reportHealthData.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/webapp/app/routes/api.v1.projects.$projectRef.environments.ts
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.in.$.ts
- apps/webapp/test/dashboardAgentRoutes.test.ts
- apps/webapp/test/reportHealthData.test.ts
- apps/webapp/app/services/routeBuilders/apiBuilder.server.ts
- apps/webapp/app/presenters/v3/reports/health/health-data.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (20)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - pnpm)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: 🛡️ E2E Auth Tests (full)
🧰 Additional context used
📓 Path-based instructions (14)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamicimport(); use dynamic imports only for unresolvable circular dependencies, genuine performance code splitting, or conditional runtime loading.
Import Trigger.dev tasks from@trigger.dev/sdk; never use@trigger.dev/sdk/v3or deprecatedclient.defineJob.
Add agentcrumbs while writing code using approved namespaces; mark lines with//@Crumbsor blocks with `// `#region` `@crumbs, and strip them before merging.
Files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use zod for validation in packages/core and apps/webapp
Files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use vitest for all tests in the Trigger.dev repository
**/*.{test,spec}.{ts,tsx}: Use Vitest exclusively and never mock dependencies; use Testcontainers for integration dependencies.
Place test files next to the source files they test.
Files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}: Access environment variables through theenvexport ofenv.server.tsinstead of directly accessingprocess.env
Use subpath exports from@trigger.dev/corepackage instead of importing from the root@trigger.dev/corepathDo not reintroduce the removed v1 execution path;
RunEngineVersion.V1branches may only reject or finalize gracefully so v3 clients receive a clean 4xx, never a 5xx.
Files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
apps/webapp/**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (.cursor/rules/webapp.mdc)
Do not import
env.server.tsdirectly or indirectly into test files; instead pass environment-dependent values through options/parameters to make code testable
Files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.ts
apps/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For apps, use
typecheckfor verification and never usebuildas the correctness check.
Files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
apps/webapp/**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
Test files must not import
app/env.server.ts; pass configuration as options instead.
Files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.ts
packages/cli-v3/src/commands/**/*
📄 CodeRabbit inference engine (packages/cli-v3/CLAUDE.md)
CLI command definitions should be located in
src/commands/
Files:
packages/cli-v3/src/commands/mint-token.ts
packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For public packages, use
buildfor verification.
Files:
packages/cli-v3/src/commands/mint-token.ts
apps/webapp/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.{ts,tsx}: For dashboard changes, visually verify the running Remix app with Chrome DevTools MCP, using snapshots, screenshots, interaction, and console-message checks as appropriate.
UseuseCallbackanduseMemoonly for context provider values, expensive derived data used as a dependency, or stable references required by dependency arrays; do not wrap ordinary event handlers or trivial computations.
Use named constants for sentinel or placeholder values instead of scattering raw string literals across comparisons.
Files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
apps/webapp/app/routes/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/routes/**/*.ts: Use Remix flat-file route conventions with dot-separated segments; for example,api.v1.tasks.$taskId.trigger.tsmaps to/api/v1/tasks/:taskId/trigger.
PAT-authenticated API routes must resolve their target organization or project within the caller's membership scope, using a membership filter or a helper such asfindProjectByReforresolveOrganizationForApiUser; RBAC authorization alone is insufficient.
Files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
apps/webapp/app/**/*.ts
📄 CodeRabbit inference engine (apps/webapp/CLAUDE.md)
apps/webapp/app/**/*.ts: Never userequest.signalto detect client disconnects. UsegetRequestAbortSignal()fromapp/services/httpAsyncStorage.server.ts, which is wired to Express response close events.
Access environment variables through theenvexport fromapp/env.server.ts; never useprocess.envdirectly.
Always use PrismafindFirstinstead offindUnique.
Always use the$transactionhelper from~/db.server, never callprisma.$transactionor$replica.$transactiondirectly. Pass isolation levels as strings, useSerializablefor correctness-critical read-then-write invariants, and guard possibly undefined helper results when a definite value is required.
Files:
apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
🧠 Learnings (17)
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-06-13T19:53:13.759Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3937
File: packages/trigger-sdk/skills/realtime-and-frontend/SKILL.md:258-260
Timestamp: 2026-06-13T19:53:13.759Z
Learning: When reviewing code that uses `trigger.dev/react-hooks`’s `useRealtimeRun`, preserve the call signature where the first argument is the full realtime handle object (not `handle.id`). This is intentional to maintain type-safety and is consistent with the official docs; do not suggest changing the first argument from the handle object to `handle.id`.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-06-17T17:13:49.929Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3948
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions.$bulkActionParam/route.tsx:48-62
Timestamp: 2026-06-17T17:13:49.929Z
Learning: In triggerdotdev/trigger.dev, within `dashboardLoader`/`dashboardAction` (or similar context resolver code) whenever you resolve an organization ID from an organization slug for RBAC/enterprise authorization scope, always read from the primary Prisma client (`prisma`), not `$replica`. Using `$replica` can hit replica-lag and cause the RBAC lookup/authorization to run without the correct org scope (bypassing intended role enforcement). Implement the slug→org lookup with `prisma.organization.findFirst(...)` (or equivalent primary-client query) and add an inline comment documenting why the primary client is required (replica lag could lead to unscoped RBAC checks).
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-05-07T12:25:18.271Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3531
File: apps/webapp/test/sentryTraceContext.server.test.ts:9-47
Timestamp: 2026-05-07T12:25:18.271Z
Learning: In the triggerdotdev/trigger.dev webapp test suite, it is acceptable to leave `createInMemoryTracing()` calls that register a global `NodeTracerProvider` without `afterEach`/`afterAll` teardown. Do not flag this as a test-ordering risk when the code follows the established pattern used across webapp tests (e.g., replication service/benchmark/backfiller tests). This is considered safe because `trace.getActiveSpan()` when called outside a `context.with(...)` block reads `AsyncLocalStorage.getStore()` (undefined when no `run()` scope exists), so it falls back to `ROOT_CONTEXT` with no attached span—regardless of which provider is registered.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.ts
📚 Learning: 2026-05-28T20:02:10.647Z
Learnt from: myftija
Repo: triggerdotdev/trigger.dev PR: 3772
File: apps/webapp/test/findOrCreateBackgroundWorker.test.ts:1-1
Timestamp: 2026-05-28T20:02:10.647Z
Learning: In the triggerdotdev/trigger.dev monorepo, for the `apps/webapp` package use the established convention of storing Vitest tests (unit, integration, and e2e) under `apps/webapp/test/` rather than colocating them next to source files. Do not flag files located in `apps/webapp/test/` as violating any rule that says to colocate tests with source.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.ts
📚 Learning: 2026-07-30T18:43:56.874Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4426
File: apps/webapp/test/memberDevEnvironments.server.test.ts:124-125
Timestamp: 2026-07-30T18:43:56.874Z
Learning: In the `apps/webapp` test suite (`apps/webapp/test/**`), respect the established test harness in `apps/webapp/test/setup.ts`: it loads `.env` and provides default values for required environment variables so that transitive imports (e.g., `~/env.server`) work without production-style wiring.
During code review, do not require dependency injection/refactoring solely to avoid this existing import path. Only introduce configuration injection if it delivers production-level value (for example, a more general `createEnvironment` abstraction that improves runtime behavior beyond test setup).
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.ts
📚 Learning: 2026-05-12T21:04:05.815Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3542
File: apps/webapp/app/components/sessions/v1/SessionStatus.tsx:1-3
Timestamp: 2026-05-12T21:04:05.815Z
Learning: In this Remix + TypeScript codebase, do not flag a server/client boundary violation when a file imports only types from a module matching `*.server`.
Specifically, it’s safe to import types using `import type { Foo } from "*.server"` or `import { type Foo } from "*.server"` because TypeScript erases type-only imports at compile time and they emit no JavaScript, so they won’t cross the Remix server/client bundle boundary.
Only raise the boundary concern for value imports (e.g., `import { Foo }` without `type`, or `import Foo`), since those produce JavaScript output.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-06-25T18:21:51.905Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4039
File: apps/webapp/app/routes/invite-revoke.tsx:0-0
Timestamp: 2026-06-25T18:21:51.905Z
Learning: During the Zod v4 migration in the triggerdotdev/trigger.dev webapp, ensure any imports from `conform-to/zod` use the Zod-4 subpath: `conform-to/zod/v4` (e.g., `import { parseWithZod } from "conform-to/zod/v4"`). Do not import from the package root `conform-to/zod`, because it is the Zod 3 implementation and may load Zod-3-only symbols (e.g., `ZodBranded`, `ZodEffects`), which can throw at module load (notably with `zod4.4.3`). This should be enforced across `apps/webapp/**/*` where helpers like `parseWithZod` and `conformZodMessage` are used.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-07-03T17:10:21.498Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 4148
File: apps/webapp/app/models/orgMember.server.ts:149-168
Timestamp: 2026-07-03T17:10:21.498Z
Learning: In triggerdotdev/trigger.dev, `User.email` (Prisma schema: `internal-packages/database/prisma/schema.prisma`) currently does NOT use `citext` and does NOT have a `lower(email)` functional unique index. Therefore, do not introduce Prisma queries like `where: { email: { equals: <value>, mode: "insensitive" } }` (or any case-insensitive lookup) against `User.email`, because it can force sequential scans of the `users` table under load. During review, ensure email is normalized (e.g., lowercased/trimmed) before both writes and subsequent lookups, and if true case-insensitive behavior/uniqueness is required, implement it via a separate app-wide migration (e.g., switch to `citext` and/or add a functional unique index with backfill) rather than bolting it onto individual feature PRs.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-05-18T14:40:02.173Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3658
File: packages/core/src/v3/realtimeStreams/manager.test.ts:1-147
Timestamp: 2026-05-18T14:40:02.173Z
Learning: In the triggerdotdev/trigger.dev repo, the policy “Never mock anything — use testcontainers instead” should only be enforced for integration tests that interact with real external services (e.g., Redis, Postgres) via actual infrastructure. For unit tests that exercise pure in-memory logic (e.g., cache semantics) it is OK to stub collaborators such as `ApiClient` using Vitest (`vi.fn()`) to assert call counts or control behavior. Do not flag `vi.fn()`-based `ApiClient` stubs in unit tests as violations of the testcontainers policy.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.ts
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/types/patRouteBuilderIdentityOnly.types.tspackages/cli-v3/src/commands/mint-token.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
📚 Learning: 2026-06-16T09:19:47.637Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3960
File: apps/webapp/test/prismaInfrastructureErrorCapture.test.ts:0-0
Timestamp: 2026-06-16T09:19:47.637Z
Learning: In this repo’s Vitest setup, `vitest.config.ts` uses `globals: true`, so identifiers like `vi`, `describe`, `it`, and `expect` are available as globals in Vitest test files. During code review, do not flag missing `vi`/`describe`/`it`/`expect` imports as a runtime error or correctness issue when they’re used in `*.test.ts/tsx` or `*.spec.ts/tsx` files. Explicit imports are still preferred for consistency, but they’re not required for runtime behavior.
Applied to files:
apps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/projectEnvironmentsBranchScope.test.tsapps/webapp/test/dashboardAgentCreateChatOrdering.test.ts
🔇 Additional comments (6)
apps/webapp/test/dashboardAgentInProxyMintFailure.test.ts (1)
1-99: LGTM!apps/webapp/test/projectEnvironmentsBranchScope.test.ts (1)
1-176: LGTM!.changeset/mint-token-cap-help.md (1)
1-5: LGTM!apps/webapp/test/types/patRouteBuilderIdentityOnly.types.ts (1)
1-27: LGTM!packages/cli-v3/src/commands/mint-token.ts (1)
30-30: LGTM!apps/webapp/test/dashboardAgentCreateChatOrdering.test.ts (1)
1-122: LGTM!
…here A start that rejects dispatched no handover and sent no message, so the chat row is taken back. A failed access-token mint is left alone: the session is live by then and removing the chat would hide a running agent.
…ide resolves flag() queried the FeatureFlag row before looking at the caller-supplied overrides, so a per-org hit still paid a round-trip. Check the override first and only fall through to the query when it fails the schema, which keeps today's resolution order intact.
… auth helper The helper checks no scopes and no capability context, so a read-only user-actor token reached an alert-channel write and every admin route behind requireAdminApiRequest. Actor-aware routes are unaffected: they authenticate the token through the route builders, which enforce its claims.
… branch The agent could not read anything on a preview or dev branch. Its environment name is derived from the environment's type, and every branch shares its parent's type — so the name identifies a family, not a row. The API's env routes are name-addressed, so a bare "preview" or "dev" resolved to the parent, and the delegated token's `environmentId` claim then correctly refused it. The guard was the detector, not the defect: the exchange was never given enough identity to resolve the environment the dashboard had selected. Name and branch are now resolved together into one address, so no caller can take the name without the branch, and both mint sites share the one type map instead of keeping a copy each. The address travels to the JWT exchange and to the three delegated-token reads that resolve by name (list_tasks, correlate_version, the repo snapshot). The env-JWT reads address the environment by id and are untouched. Second, and why nobody saw the first: the exchange reported the same "no environment" for a genuine absence and for any failure, and cached the failure for the whole turn. Following the queue live-read precedent, only a missing environment is stated as one; anything else says the read didn't land and carries its status.
A missing telemetry feed said the run aggregates were a point-in-time snapshot; they are measured over the window either way, and what is actually unknown is how current they are. An unmeasured queue depth blamed throughput, which the report does measure, and contradicted its own headline. Tests pin the claim rather than the wording.
… flow finding An unmeasurable depth made the whole flow finding unassessable, so a critical start latency measured off runs showed as crit in format=json while the rendered report said 'Flow unknown ... nothing to do' and dropped the row that earned it. The depth still buys no cause, no attribution and no drain ETA, but the measured symptom is now reported.
The prompt requires render_view and the schema union allows actions, investigation and report blocks, but ViewBlocks only had cases for diagnosis and chart, so those three rendered as an empty div. Adds the three renderers, the block-envelope latest-wins resolution the switch keys on, and a contract test that reads the block types off viewBlockSchema, so a new union member fails until it has a renderer.
…s current name `curateReport` still read `facts.staleReason`, renamed to `untrustworthyReason` in dc3b502 and split into telemetry_stale / telemetry_absent / flow_unmeasured. The read had been undefined since, so the agent got "untrustworthy" with no why, and the prompt still told it every such case was stale telemetry. `facts` is `z.record(z.unknown())`, so nothing typechecked the key. The new test goes presenter -> reports route JSON -> curateReport without naming a facts key on the way in, and asserts curation carries every key the presenter emits.
…failed tool call in evals
…ally holds `collectDurableState` read `data-view` parts only, but `render_view` writes every investigation card as a `tool-render_view` part, so compaction pinned nothing and an open card could be summarised away. It now resolves cards through `latestCards`, the resolver the panel and the watch actions already use: highest revision per id wins whatever order the renders arrive in, so a stale `in_progress` render landing after the settling one no longer reopens a closed card. `latestCards` reads host-written `data-view` blocks too, matching the panel.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
apps/webapp/app/components/dashboard-agent/view-actions.test.ts (1)
53-60: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer behavior assertions over exact source strings.
Lines 54-55 and Line 59 assert exact formatted source text of
ActionsBlock.tsx. A Prettier reflow or a parameter rename breaks these tests without any behavior change. The purity checks at Lines 62-66 are regex-based and stay stable, so only the substring assertions carry this cost. Consider renderingActionsBlockand asserting the emitted intent instead.apps/webapp/test/dashboardAgentInvestigationWinner.test.ts (1)
48-62: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd a tie-break case for equal revisions.
winningInvestigationOccurrencescompares withref.revision >= current.revision, so on equal revisions the later message wins. No test covers that branch. A future change to>would pass this suite and silently flip which occurrence renders.💚 Proposed additional test
+ it("prefers the later render when two revisions tie", () => { + const winners = winningInvestigationOccurrences([ + investigationMessage({ id: "inv_3", title: "tie", outcome: "in_progress", revision: 2 }), + investigationMessage({ id: "inv_3", title: "tie again", outcome: "concluded", revision: 2 }), + ]); + + expect(winners.get("inv_3")).toBe("msg-inv_3-2:0"); + });Note that both helper calls produce the same message id here, so give the second one a distinct id if you want the assertion to discriminate.
apps/webapp/app/components/dashboard-agent/agent-badges.tsx (1)
163-173: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGive
AgentStatusIconan accessible name option.
AgentStatusIconrenders an icon alone. It has noaria-hiddenand no accessible name. If a caller uses it as the only indicator of a state, assistive technology conveys nothing. If it always sits next to a text label, mark it decorative instead.♿ Proposed change
export function AgentStatusIcon({ tone, icon: Icon, className, + label, }: { tone: AgentTone; icon: IconComponent; className?: string; + /** Set when the icon is the only carrier of the state. Omit when a text label sits beside it. */ + label?: string; }) { - return <Icon className={cn("size-4 shrink-0", TONE_ICON_COLOR[tone], className)} />; + return ( + <span role={label ? "img" : undefined} aria-label={label} aria-hidden={label ? undefined : true}> + <Icon className={cn("size-4 shrink-0", TONE_ICON_COLOR[tone], className)} /> + </span> + ); }
IconComponentat Line 34 accepts onlyclassName, so the wrapper element carries the ARIA attributes.apps/webapp/app/services/dashboardAgentEnvironmentAddress.server.ts (1)
17-37: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winType the name map on
RuntimeEnvironmentType.
ENV_NAME_BY_TYPEusesRecord<string, string>and accepts{ type: string }, so an invalid or future environment type can pass without a compile error and returnenvironmentName: undefined. Use the existing Prisma union viaimport type { RuntimeEnvironmentType } from "@trigger.dev/database", then keep the map keyed byRuntimeEnvironmentTypeso the compiler requires a name for every environment type.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 708e1764-0eb1-4f3e-abda-fbfb9f476494
⛔ Files ignored due to path filters (1)
internal-packages/dashboard-agent/src/__snapshots__/prompt-prefix.test.ts.snapis excluded by!**/*.snap
📒 Files selected for processing (51)
.server-changes/dashboard-agent.mdapps/webapp/app/components/dashboard-agent/ActionsBlock.tsxapps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsxapps/webapp/app/components/dashboard-agent/InvestigationCard.test.tsapps/webapp/app/components/dashboard-agent/InvestigationCard.tsxapps/webapp/app/components/dashboard-agent/ReportView.test.tsapps/webapp/app/components/dashboard-agent/ReportView.tsxapps/webapp/app/components/dashboard-agent/agent-badges.tsxapps/webapp/app/components/dashboard-agent/agent-card.tsxapps/webapp/app/components/dashboard-agent/chat-layout.tsxapps/webapp/app/components/dashboard-agent/report-spark.test.tsapps/webapp/app/components/dashboard-agent/report-spark.tsapps/webapp/app/components/dashboard-agent/report-sparkline.tsxapps/webapp/app/components/dashboard-agent/view-actions.test.tsapps/webapp/app/components/dashboard-agent/view-actions.tsapps/webapp/app/components/dashboard-agent/view-blocks.test.tsapps/webapp/app/components/dashboard-agent/view-blocks.tsapps/webapp/app/components/dashboard-agent/view-catalog.test.tsapps/webapp/app/components/dashboard-agent/view-catalog.tsxapps/webapp/app/presenters/v3/reports/health/flow.tsapps/webapp/app/presenters/v3/reports/report-layout.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.in.$.tsapps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.tsapps/webapp/app/services/dashboardAgent.server.tsapps/webapp/app/services/dashboardAgentEnvironmentAddress.server.tsapps/webapp/app/services/personalAccessToken.server.tsapps/webapp/test/dashboardAgentClientMetadata.test.tsapps/webapp/test/dashboardAgentInProxyMintFailure.test.tsapps/webapp/test/dashboardAgentInvestigationWinner.test.tsapps/webapp/test/reportCurationTrust.test.tsapps/webapp/test/reportHealth.test.tsapps/webapp/test/reportTrust.test.tsapps/webapp/test/uatEnvironmentClaim.test.tsapps/webapp/test/userActorPatOnlyBoundary.test.tsapps/webapp/test/userActorTokenClaimsAndScopes.test.tsinternal-packages/dashboard-agent/package.jsoninternal-packages/dashboard-agent/src/agent-runtime.tsinternal-packages/dashboard-agent/src/compaction.test.tsinternal-packages/dashboard-agent/src/compaction.tsinternal-packages/dashboard-agent/src/eval-error-category.test.tsinternal-packages/dashboard-agent/src/eval-policy.tsinternal-packages/dashboard-agent/src/eval-redaction.test.tsinternal-packages/dashboard-agent/src/eval-turn.tsinternal-packages/dashboard-agent/src/tool-api-branch.test.tsinternal-packages/dashboard-agent/src/tool-api-client.tsinternal-packages/dashboard-agent/src/tool-api.tsinternal-packages/dashboard-agent/src/tool-context.tsinternal-packages/dashboard-agent/src/tool-curation.tsinternal-packages/dashboard-agent/src/tool-schemas.tsinternal-packages/dashboard-agent/src/tool-source-ledger.tsinternal-packages/dashboard-agent/src/tools.ts
💤 Files with no reviewable changes (2)
- apps/webapp/test/dashboardAgentInProxyMintFailure.test.ts
- apps/webapp/app/services/dashboardAgent.server.ts
🚧 Files skipped from review as they are similar to previous changes (19)
- apps/webapp/test/reportTrust.test.ts
- apps/webapp/test/userActorTokenClaimsAndScopes.test.ts
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.in.$.ts
- internal-packages/dashboard-agent/src/compaction.test.ts
- internal-packages/dashboard-agent/src/tool-context.ts
- internal-packages/dashboard-agent/package.json
- .server-changes/dashboard-agent.md
- apps/webapp/test/uatEnvironmentClaim.test.ts
- internal-packages/dashboard-agent/src/eval-redaction.test.ts
- internal-packages/dashboard-agent/src/compaction.ts
- apps/webapp/app/presenters/v3/reports/report-layout.ts
- internal-packages/dashboard-agent/src/eval-turn.ts
- internal-packages/dashboard-agent/src/tool-source-ledger.ts
- apps/webapp/app/routes/resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.dashboard-agent.ts
- internal-packages/dashboard-agent/src/tools.ts
- internal-packages/dashboard-agent/src/agent-runtime.ts
- internal-packages/dashboard-agent/src/tool-curation.ts
- apps/webapp/app/presenters/v3/reports/health/flow.ts
- internal-packages/dashboard-agent/src/tool-schemas.ts
| if (!spec) return renderPart(part, i); | ||
| const blocks = withoutSupersededInvestigations( | ||
| spec.blocks, | ||
| `${message.id}:${i}`, | ||
| investigationWinners | ||
| ); | ||
| if (blocks.length === 0) return null; | ||
| return <ViewBlocks key={i} blocks={blocks as never} />; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Forward the dashboard-agent view handlers.
Line 100 renders ViewBlocks without onIntent. ActionsBlock then returns null, and InvestigationCard cannot render its action controls. URI resolution, report page paths, and the per-part answered state are also not forwarded.
Add these host inputs to DashboardAgentMessages and pass them to ViewBlocks. Set answered from answerContinuesAfter(message.parts, i) for each rendered view part.
| export async function apiGet( | ||
| origin: string, | ||
| path: string, | ||
| token: string, | ||
| branch?: string | ||
| ): Promise<FetchResult> { | ||
| const headers: Record<string, string> = { | ||
| Authorization: `Bearer ${token}`, | ||
| Accept: "application/json", | ||
| }; | ||
| if (branch) headers["x-trigger-branch"] = branch; | ||
| const res = await fetch(`${origin}${path}`, { headers }); | ||
| if (!res.ok) return { ok: false, status: res.status }; | ||
| return { ok: true, data: await res.json() }; | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how tool execute rejections are handled by the agent runtime.
rg -nP -C6 'execute|onError|toolCallError|catch' internal-packages/dashboard-agent/src/agent-runtime.ts
rg -nP -C4 '\bapiGet\(' internal-packages/dashboard-agent/srcRepository: triggerdotdev/trigger.dev
Length of output: 7322
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate files =="
git ls-files internal-packages/dashboard-agent/src | rg '(^internal-packages/dashboard-agent/src/tool-api-client\.ts$|^internal-packages/dashboard-agent/src/tool-api\.ts$|^internal-packages/dashboard-agent/src/tool-source-ledger\.ts$|^internal-packages/dashboard-agent/src/agent-.*\.ts$|^internal-packages/dashboard-agent/src/.*runtime.*\.ts$)'
echo
echo "== tool-api-client relevant sections =="
sed -n '1,230p' internal-packages/dashboard-agent/src/tool-api-client.ts | nl -ba
echo
echo "== tool-api relevant execute sections =="
sed -n '60,130p' internal-packages/dashboard-agent/src/tool-api.ts | nl -ba
sed -n '400,440p' internal-packages/dashboard-agent/src/tool-api.ts | nl -ba
echo
echo "== tool-source-ledger relevant sections =="
sed -n '1,90p' internal-packages/dashboard-agent/src/tool-source-ledger.ts | nl -ba
echo
echo "== look for execute rejection handling in dashboard agent =="
rg -n -C5 'execute|agent|run|onError|error|catch|toolCallError|tool-call-error|schema-usage-error|unhandled' internal-packages/dashboard-agent/src/agent*.ts internal-packages/dashboard-agent/src/agent-runtime.ts internal-packages/dashboard-agent/src/*.ts 2>/dev/null || true
echo
echo "== read all agent runtime files =="
for f in internal-packages/dashboard-agent/src/agent*.ts internal-packages/dashboard-agent/src/agent-runtime.ts; do
[ -f "$f" ] && { echo "--- $f"; wc -l "$f"; sed -n '1,220p' "$f" | nl -ba; }
doneRepository: triggerdotdev/trigger.dev
Length of output: 488
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' "tool-api-client.ts:"
sed -n '1,230p' internal-packages/dashboard-agent/src/tool-api-client.ts
printf '%s\n' "tool-api.ts execute sections:"
sed -n '70,125p' internal-packages/dashboard-agent/src/tool-api.ts
sed -n '405,430p' internal-packages/dashboard-agent/src/tool-api.ts
printf '%s\n' "tool-source-ledger.ts:"
sed -n '1,90p' internal-packages/dashboard-agent/src/tool-source-ledger.ts
printf '%s\n' "agent/runtime search:"
rg -n -C4 'execute|agent|run\(|onError|error|catch|toolCallError|tool-call-error|schema-usage-error|unhandled' internal-packages/dashboard-agent/src 2>/dev/null || true
printf '%s\n' "agent-runtime.ts:"
sed -n '1,260p' internal-packages/dashboard-agent/src/agent-runtime.tsRepository: triggerdotdev/trigger.dev
Length of output: 50382
Handle transport failures in apiGet like the other network paths.
apiGet calls fetch and res.json() without catching failures. Connection/DNS/abort failures make the tool execute promise reject, while callers only read result.ok to decide between structured success or error responses. exchangeEnvJwt() and postQuery() already catch this class of failure and return { ok: false, status }, so apiGet() should do the same for consistent degraded behavior.
The watch tables and the 0002 migration stay here, so the committed drizzle snapshot keeps describing them. The four contract types the schema annotates are widened here and re-narrowed where the watch code lands.
| function investigationBlocksFor(part: UIMessage["parts"][number]): unknown[] | null { | ||
| return viewSpecFor(part)?.blocks ?? hostViewBlocks(part); | ||
| } |
There was a problem hiding this comment.
🔍 Host-written data-view cards can suppress the tool-rendered card they compete with
investigationBlocksFor counts investigation blocks from both tool-render_view output and host-written data-view parts (apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx:26-34), so a data-view occurrence can win the highest-revision contest. The render pass, however, only ever draws blocks from viewSpecFor (DashboardAgentMessages.tsx:92-100); a data-view part is handed to renderPart, and a message containing only data-view parts doesn't even take this branch (it falls through to MessageBubble). If a host-written card ever carries the winning revision for an id, the tool-rendered occurrence is filtered away and nothing renders that investigation. Worth confirming against the follow-up UI PR whether data-view investigation blocks are actually produced in the panel's transcript.
Was this helpful? React with 👍 or 👎 to provide feedback.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (5)
internal-packages/dashboard-agent-db/src/queries.ts (5)
59-110: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftPreserve legacy transcripts during the read-path migration.
getChatMessagesandcountUserMessagesnow read onlychatMessages. The additive migration createschat_messagesbut does not copy existingchats.messagesJSONB. A pre-existing chat can therefore load as an empty transcript and contribute zero user messages to quota. Add a backfill with contiguous positions and an updatednextMessagePosition, or keep a legacy fallback until backfill completes. (raw.githubusercontent.com)🔎 Verify the upgrade path
#!/usr/bin/env bash set -euo pipefail rg -n -C 3 \ 'chat_messages|chats.*messages|jsonb_array_elements|INSERT INTO' \ internal-packages/dashboard-agent-db/drizzle \ internal-packages/dashboard-agent-db/src
122-133: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winExclude soft-deleted chats from session reads.
getSessiondoes not filterchats.deletedAt.getChatMessagestreats a deleted chat as missing, but the route still returns the separately loaded session. This can exposepublicAccessTokenafter deletion. AddisNull(chats.deletedAt)to the session predicate. (raw.githubusercontent.com)🔒 Proposed fix
and( eq(chatSessions.chatId, params.chatId), eq(chats.userId, params.userId), eq(chats.organizationId, params.organizationId), + isNull(chats.deletedAt), )
176-194: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep soft-deleted chats immutable.
renameChatandsetChatPinneddo not includeisNull(chats.deletedAt). The dashboard-agent action calls both without a precedingchatExistscheck, so a stale client can mutate a deleted row and receive success. Add the deleted-chat predicate to both updates. (raw.githubusercontent.com)🐛 Proposed fix
and( eq(chats.id, params.chatId), eq(chats.userId, params.userId), eq(chats.organizationId, params.organizationId), + isNull(chats.deletedAt), )Apply the same predicate to both update statements.
Also applies to: 207-221
499-604: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftMake settlement persistence idempotent across retries.
persistTurncallsupsertInvestigationRevisionfor every pending settlement. The upsert always increments the revision for an existing ID. A retry after commit therefore creates a new revision and a new revision-based card ID. Theexistingset cannot match the older card. Use a per-turn idempotency key or an expected revision, and reuse the original settlement revision. Add a retry test that asserts one closing card. (raw.githubusercontent.com)Also applies to: 606-696
868-875: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winRoll back when the closing card cannot be appended.
appendChatMessageOnceByChatIdreturnsfalsefor both duplicate messages and missing or deleted chats.settleInvestigationAndCloseCardcommits the investigation settlement for both cases. If deletion wins the race, the investigation becomes terminal without a transcript card. Confirm that the message already exists whenclosedis false; otherwise throw to roll back. (raw.githubusercontent.com)🐛 Proposed fix
const closed = await appendChatMessageOnceByChatId(tx, { chatId: params.chatId, message, }); + if (!closed) { + const existing = await storedMessageById(tx, { + chatId: params.chatId, + messageId: message.id, + }); + if (!existing) { + throw new Error(`Investigation ${settled.id} settled without a closing card`); + } + } return { settled, closed };
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e1aa1491-ac5f-4918-ba66-4e997564f64d
📒 Files selected for processing (12)
.gitattributesapps/webapp/test/dashboardAgentCreateChatOrdering.test.tsapps/webapp/test/dashboardAgentTranscriptStore.test.tsinternal-packages/dashboard-agent-contracts/src/index.tsinternal-packages/dashboard-agent-db/src/ids.tsinternal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.tsinternal-packages/dashboard-agent-db/src/watch-schema.tsinternal-packages/dashboard-agent/src/dashboard-agent.test.tsinternal-packages/dashboard-agent/src/dashboard-agent.tsinternal-packages/dashboard-agent/src/eval-error-category.test.tsinternal-packages/dashboard-agent/src/eval-redaction.test.ts
💤 Files with no reviewable changes (2)
- internal-packages/dashboard-agent-contracts/src/index.ts
- internal-packages/dashboard-agent-db/src/ids.ts
🚧 Files skipped from review as they are similar to previous changes (6)
- apps/webapp/test/dashboardAgentCreateChatOrdering.test.ts
- internal-packages/dashboard-agent/src/eval-error-category.test.ts
- internal-packages/dashboard-agent-db/src/watch-schema.ts
- apps/webapp/test/dashboardAgentTranscriptStore.test.ts
- internal-packages/dashboard-agent/src/eval-redaction.test.ts
- internal-packages/dashboard-agent/src/dashboard-agent.ts
📜 Review details
⏰ Context from checks skipped due to timeout. (21)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (12, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (8, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (9, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (10, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (5, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (7, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (11, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (4, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (1, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (2, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (3, 12)
- GitHub Check: webapp / 🧪 Unit Tests: Webapp (6, 12)
- GitHub Check: internal / 🧪 Unit Tests: Internal
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (warp-windows-latest-x64-8x - npm)
- GitHub Check: packages / 🧪 Unit Tests: Packages (3, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (2, 3)
- GitHub Check: packages / 🧪 Unit Tests: Packages (1, 3)
- GitHub Check: e2e-webapp / 🧪 E2E Tests: Webapp
- GitHub Check: 🛡️ E2E Auth Tests (full)
- GitHub Check: Analyze (javascript-typescript)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.{ts,tsx}: Use types over interfaces for TypeScript
Avoid using enums; prefer string unions or const objects instead
**/*.{ts,tsx}: Prefer static imports over dynamicimport(); use dynamic imports only for unresolvable circular dependencies, genuine performance code splitting, or conditional runtime loading.
Import Trigger.dev tasks from@trigger.dev/sdk; never use@trigger.dev/sdk/v3or deprecatedclient.defineJob.
Add agentcrumbs while writing code using approved namespaces; mark lines with//@Crumbsor blocks with `// `#region` `@crumbs, and strip them before merging.
Files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Use function declarations instead of default exports
Files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
**/*.ts
📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc)
**/*.ts: When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs
Do not use high-cardinality attributes in OTEL metrics such as UUIDs/IDs (envId, userId, runId, projectId, organizationId), unbounded integers (itemCount, batchSize, retryCount), timestamps (createdAt, startTime), or free-form strings (errorMessage, taskName, queueName)
When exporting OTEL metrics via OTLP to Prometheus, be aware that the exporter automatically adds unit suffixes to metric names (e.g., 'my_duration_ms' becomes 'my_duration_ms_milliseconds', 'my_counter' becomes 'my_counter_total'). Account for these transformations when writing Grafana dashboards or Prometheus queries
Files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
internal-packages/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
For internal packages, use
typecheckfor verification and never usebuildas the correctness check.
Files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
🧠 Learnings (14)
📚 Learning: 2026-04-13T21:43:58.707Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3368
File: apps/webapp/app/services/taskIdentifierRegistry.server.ts:49-58
Timestamp: 2026-04-13T21:43:58.707Z
Learning: In `apps/webapp/app/services/taskIdentifierRegistry.server.ts` (and closely related task registry/sync code in this module), an empty deployment (zero tasks) is not a realistic/expected scenario. Do not flag or require special handling or fallback logic for missing empty-task cases in this registry/sync flow.
Applied to files:
.gitattributes
📚 Learning: 2026-04-23T13:26:27.529Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3430
File: apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts:0-0
Timestamp: 2026-04-23T13:26:27.529Z
Learning: In this codebase’s SSE implementation (notably `apps/webapp/app/utils/sse.ts` and its SSE callers such as `apps/webapp/app/presenters/v3/RunStreamPresenter.server.ts`), do not treat `.abort()` vs `.abort("<reason string>")` differences as a heap retention/memory leak issue on Node 20. Heap deltas are identical once `AbortSignal.any` is not involved; the investigated SSE memory leak root cause was `AbortSignal.any` (via FinalizationRegistry behavior on abort). When reviewing, focus on whether `AbortSignal.any` (or similar signal-combining/finalization patterns) is used in the SSE abort path rather than the presence/contents of the abort reason string. Named abort-reason sentinel constants (e.g., `ABORT_REASON_*`) are for readability/style only, not correctness.
Applied to files:
.gitattributes
📚 Learning: 2026-04-30T21:28:35.705Z
Learnt from: samejr
Repo: triggerdotdev/trigger.dev PR: 3473
File: internal-packages/database/prisma/schema.prisma:59-60
Timestamp: 2026-04-30T21:28:35.705Z
Learning: In this repo, session-duration lower-bound validation is intentionally enforced only at the app layer (via `isAllowedSessionDuration` and `SESSION_DURATION_OPTIONS`), where the minimum allowed value is 300 seconds (5 minutes). `User.sessionDuration` is `Int` (non-nullable) and `Organization.maxSessionDuration` is `Int?` (nullable). There are currently no DB-level `CHECK` constraints for these fields anywhere in the migrations, so future reviews should not flag the absence of DB `CHECK` constraints on these session-duration fields—focus validation on the app-layer logic instead.
Applied to files:
.gitattributes
📚 Learning: 2026-05-12T06:43:09.116Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3558
File: internal-packages/run-engine/src/run-queue/index.ts:420-424
Timestamp: 2026-05-12T06:43:09.116Z
Learning: In internal-packages/run-engine/src/run-queue/index.ts, the RunQueue “read-path” counter/concurrency methods (e.g., lengthOfQueue/lengthOfQueues/currentConcurrencyOfQueues) intentionally “fail open” on transient Redis pipeline result errors. If you see pipeline result errors (e.g., baseErr/ctrErr) being coerced to 0 (treating Redis command errors the same as missing keys) rather than surfaced/thrown, do not flag it as a bug and do not suggest propagating those errors in future reviews.
Applied to files:
.gitattributes
📚 Learning: 2026-05-19T21:04:49.990Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3671
File: packages/trigger-sdk/src/v3/ai.ts:724-769
Timestamp: 2026-05-19T21:04:49.990Z
Learning: In packages/trigger-sdk/src/v3/ai.ts, when reviewing recovery/replay of turns, do NOT flag “lost metadata on recovered turns” if the recovered turns are reconstructed via the recovery boot queue that maps metadata by message.id (using replaySessionInTail’s {message, metadata, seqNum} per session.in record). If a message has no matching session.in record (e.g., hook-synthesized messages), the code intentionally falls back to the current boot payload’s metadata—this fallback is expected behavior.
Applied to files:
.gitattributes
📚 Learning: 2026-03-22T13:26:12.060Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3244
File: apps/webapp/app/components/code/TextEditor.tsx:81-86
Timestamp: 2026-03-22T13:26:12.060Z
Learning: In the triggerdotdev/trigger.dev codebase, do not flag `navigator.clipboard.writeText(...)` calls for `missing-await`/`unhandled-promise` issues. These clipboard writes are intentionally invoked without `await` and without `catch` handlers across the project; keep that behavior consistent when reviewing TypeScript/TSX files (e.g., usages like in `apps/webapp/app/components/code/TextEditor.tsx`).
Applied to files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
📚 Learning: 2026-03-22T19:24:14.403Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3187
File: apps/webapp/app/v3/services/alerts/deliverErrorGroupAlert.server.ts:200-204
Timestamp: 2026-03-22T19:24:14.403Z
Learning: In the triggerdotdev/trigger.dev codebase, webhook URLs are not expected to contain embedded credentials/secrets (e.g., fields like `ProjectAlertWebhookProperties` should only hold credential-free webhook endpoints). During code review, if you see logging or inclusion of raw webhook URLs in error messages, do not automatically treat it as a credential-leak/secrets-in-logs issue by default—first verify the URL does not contain embedded credentials (for example, no username/password in the URL, no obvious secret/token query params or fragments). If the URL is credential-free per this project’s conventions, allow the logging.
Applied to files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma error P1001 ("Can't reach database server") in TypeScript, don’t assume a single error shape. Prisma can surface P1001 via two different error classes/fields: `PrismaClientKnownRequestError` exposes it as `err.code === "P1001"` (common during mid-query connection drops), while `PrismaClientInitializationError` exposes it as `err.errorCode === "P1001"` (common on client startup failure). Therefore, predicates should use `err.code === "P1001" || err.errorCode === "P1001"`. Do not flag `err.code === "P1001"` as “unreachable/never matches,” as it is expected in production.
Applied to files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
📚 Learning: 2026-05-18T08:21:27.694Z
Learnt from: d-cs
Repo: triggerdotdev/trigger.dev PR: 3632
File: apps/webapp/sentry.server.ts:4-21
Timestamp: 2026-05-18T08:21:27.694Z
Learning: When handling Prisma errors for P1001 ("Can't reach database server"), do not assume it only appears under a single property name. Prisma may surface P1001 via either `PrismaClientKnownRequestError` (`err.code === "P1001"`, e.g., mid-query connection drops) or `PrismaClientInitializationError` (`err.errorCode === "P1001"`, e.g., client startup connection failure). To reliably detect the condition, check `err.code === "P1001" || err.errorCode === "P1001"`, and avoid review rules that would incorrectly flag `err.code === "P1001"` as unreachable/never-matching.
Applied to files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
📚 Learning: 2026-06-13T19:53:13.759Z
Learnt from: ericallam
Repo: triggerdotdev/trigger.dev PR: 3937
File: packages/trigger-sdk/skills/realtime-and-frontend/SKILL.md:258-260
Timestamp: 2026-06-13T19:53:13.759Z
Learning: When reviewing code that uses `trigger.dev/react-hooks`’s `useRealtimeRun`, preserve the call signature where the first argument is the full realtime handle object (not `handle.id`). This is intentional to maintain type-safety and is consistent with the official docs; do not suggest changing the first argument from the handle object to `handle.id`.
Applied to files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
📚 Learning: 2026-06-17T17:13:49.929Z
Learnt from: matt-aitken
Repo: triggerdotdev/trigger.dev PR: 3948
File: apps/webapp/app/routes/_app.orgs.$organizationSlug.projects.$projectParam.env.$envParam.bulk-actions.$bulkActionParam/route.tsx:48-62
Timestamp: 2026-06-17T17:13:49.929Z
Learning: In triggerdotdev/trigger.dev, within `dashboardLoader`/`dashboardAction` (or similar context resolver code) whenever you resolve an organization ID from an organization slug for RBAC/enterprise authorization scope, always read from the primary Prisma client (`prisma`), not `$replica`. Using `$replica` can hit replica-lag and cause the RBAC lookup/authorization to run without the correct org scope (bypassing intended role enforcement). Implement the slug→org lookup with `prisma.organization.findFirst(...)` (or equivalent primary-client query) and add an inline comment documenting why the primary client is required (replica lag could lead to unscoped RBAC checks).
Applied to files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
📚 Learning: 2026-06-23T13:04:21.413Z
Learnt from: carderne
Repo: triggerdotdev/trigger.dev PR: 4023
File: apps/webapp/app/services/upsertBranch.server.ts:14-18
Timestamp: 2026-06-23T13:04:21.413Z
Learning: In TypeScript, it’s valid to `import { type X }` and then use `typeof X` in a type-only position, e.g. `type Alias = z.infer<typeof X>`. The `type` modifier suppresses the runtime import, but the type checker still has the full exported type so `z.infer<typeof X>` can resolve correctly. In code reviews, don’t flag this as a TypeScript compile error as long as `typeof X` is used in a type context (e.g., with `z.infer`, `type` aliases, generics), not as a runtime value.
Applied to files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
📚 Learning: 2026-06-04T18:16:35.386Z
Learnt from: nicktrn
Repo: triggerdotdev/trigger.dev PR: 3836
File: apps/supervisor/src/backpressure/backpressureMonitor.ts:3-5
Timestamp: 2026-06-04T18:16:35.386Z
Learning: When reviewing TypeScript in this repo, apply the rule “prefer type aliases over interfaces” only to data/object shapes and union/intersection type modeling. If an interface is being used as a behavioral contract for collaborators to implement (e.g., method-shape interfaces that define required behavior, such as `BackpressureLogger` / `BackpressureSignalSource` in `apps/supervisor/src/backpressure/backpressureMonitor.ts`), keep it as an `interface` and do not flag it as a type-alias-vs-interface violation.
Applied to files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
📚 Learning: 2026-06-09T17:58:04.699Z
Learnt from: 0ski
Repo: triggerdotdev/trigger.dev PR: 3879
File: apps/webapp/app/models/vercelIntegration.server.ts:619-630
Timestamp: 2026-06-09T17:58:04.699Z
Learning: In this codebase, outbound raw `fetch` calls should typically rely on Node/undici’s default request timeout (about ~300s) rather than adding a per-call `AbortController` + `setTimeout` wrapper inside individual functions (e.g. in files like `apps/webapp/app/models/vercelIntegration.server.ts`). During code review, do not flag the absence of a per-call timeout on a single `fetch` as an issue; if per-call timeouts are needed, they should be implemented via a codebase-wide convention (e.g., a shared fetch wrapper or documented pattern) rather than ad-hoc per-function changes.
Applied to files:
internal-packages/dashboard-agent-db/src/internal.tsinternal-packages/dashboard-agent-db/src/queries.ts
🔇 Additional comments (6)
.gitattributes (1)
1-3: LGTM!internal-packages/dashboard-agent-db/src/internal.ts (1)
1-7: LGTM!internal-packages/dashboard-agent-db/src/queries.ts (4)
469-497: Duplicate: requirerolein both append helper types.The public signatures still allow
message: { id: string }, butappendOneMessagethrows whenroleis missing. This is the same contract issue reported in the prior review. The supplied file still contains the old signatures. (raw.githubusercontent.com)
1-19: LGTM!Also applies to: 32-32, 153-153, 223-422
862-875: 🗄️ Data Integrity & IntegrationVerify the stale-settlement chat binding.
settleInvestigationAndCloseCardaccepts bothidandchatId, butsettleInvestigationAsInconclusivefilters only by investigation ID and outcome. A mismatched pair can settle one investigation and append its card to another chat. IncludechatIdin the update predicate, or verify every caller passes the investigation's own chat ID. (raw.githubusercontent.com)#!/usr/bin/env bash set -euo pipefail rg -n -C 8 \ '\bsettleInvestigationAndCloseCard\s*\(' \ internal-packages apps \ --glob '*.ts' \ --glob '*.tsx'
698-860: LGTM!Also applies to: 937-1069
| error?: Error; | ||
| }) { | ||
| const rootRef = useAutoScrollToBottom([messages, isThinking]); | ||
| const investigationWinners = winningInvestigationOccurrences(messages); |
There was a problem hiding this comment.
🟡 Every chat message re-renders on each streaming update
A freshly built lookup table is handed to each message on every render (winningInvestigationOccurrences(messages) at apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx:120), so the per-message render skipping no longer applies and the whole conversation is redrawn on every streamed chunk.
Impact: Long conversations get progressively janglier while the assistant is typing.
New Map identity defeats React.memo
DashboardAgentMessageBubble is wrapped in memo (DashboardAgentMessages.tsx:78) and its file comment states that "streaming memoization is preserved for the common case". winningInvestigationOccurrences returns a brand-new Map on every render of DashboardAgentMessages, and it is passed as the investigationWinners prop to every bubble (DashboardAgentMessages.tsx:129). memo's default shallow comparison sees a different object identity each time, so no bubble is ever skipped during streaming.
Memoizing the map (useMemo keyed on messages) restores the previous behaviour.
Prompt for agents
`winningInvestigationOccurrences(messages)` in apps/webapp/app/components/dashboard-agent/DashboardAgentMessages.tsx creates a new Map on every render and is passed to every memoized DashboardAgentMessageBubble, which defeats the memoization the file comment says it preserves. Wrap the computation in useMemo keyed on `messages` (this is derived data used as a dependency of a memoized child, which the repo's React guidance allows).
Was this helpful? React with 👍 or 👎 to provide feedback.
| return; | ||
| } | ||
|
|
||
| // A user-actor token authenticates as the user wherever a PAT does. | ||
| // The plugin verifies it (identity path → no org context to floor against). | ||
| // PAT-only: this helper checks no scopes and no capability context, and its callers include | ||
| // actions and the admin gate. A delegated token is refused at the entrance and reaches the | ||
| // API through the actor-aware routes instead, which do enforce its claims. | ||
| if (isUserActorToken(token)) { | ||
| const result = await rbac.authenticateUserActor(request, {}); | ||
| return result.ok ? { userId: result.userId } : undefined; | ||
| logger.warn("Rejected a user-actor token at the PAT-only authentication helper"); | ||
| return; |
There was a problem hiding this comment.
🔍 Rejecting user-actor tokens at the PAT helper is a behavioral break for existing callers
authenticateApiRequestWithPersonalAccessToken previously accepted a delegated user-actor token via rbac.authenticateUserActor; it now logs a warning and returns undefined. Any route that authenticates through this helper and was previously reachable by a CLI/MCP-minted UAT will start returning 401. The PR intends this ("refused at the entrance"), and the actor-aware routes were migrated, but it would be worth an explicit audit of every caller of this helper to confirm none of them is on a path an existing UAT client (CLI mint-token, MCP) relies on today.
Was this helpful? React with 👍 or 👎 to provide feedback.
…ken request reading as an answer An investigation card could vanish from a reply that contained step separators: the winning revision was keyed by its position in the untouched reply, while rendering walked the stripped parts. Both now index the same stripped list. A host-written `data-view` card counted towards winning a revision but was never drawn as a card, so it could suppress the tool-rendered card it competed with into nothing. Both carriers now render. `apiGet` left `fetch` and `res.json()` unguarded, so a connection reset threw out of the tool, and a transport failure that did land could read as a definite 404. It now returns a transport failure the way the sibling paths do. Also: guard the card's Hypotheses section on length, as Evidence already is, and move the reports auth resource out of the serializer so rendering a report doesn't pull the route builder in.
| { | ||
| method: "POST", | ||
| body: CreateOrgRequestBody, | ||
| authorization: { action: "manage", resource: () => ({ type: "organization" }) }, |
There was a problem hiding this comment.
🔍 New manage gate on org creation may reject ordinary PATs under the closed-source RBAC plugin
The action now declares authorization: { action: "manage", resource: () => ({ type: "organization" }) }, but the route has no context function, so rbac.authenticatePat(request, {}) is called with an empty { organizationId, projectId }. On OSS this is harmless (authenticatePat returns permissiveAbility in internal-packages/rbac/src/fallback.ts), but on cloud the ability is role-floored against the supplied org context — and there is no org yet, by definition. If the plugin cannot resolve a role from an empty context it will deny manage organization for every PAT, which would break POST /api/v1/orgs outright for real users. The new tests in apps/webapp/test/contextlessPatRoutes.test.ts only exercise the user-actor branch with a stubbed controller, so they would not catch this. Worth confirming against the actual plugin's behaviour for a contextless manage check before merging.
Was this helpful? React with 👍 or 👎 to provide feedback.
| ALTER TABLE "trigger_dashboard_agent"."chats" ADD COLUMN IF NOT EXISTS "last_read_at" timestamp with time zone; | ||
| --> statement-breakpoint | ||
| ALTER TABLE "trigger_dashboard_agent"."chats" ADD COLUMN IF NOT EXISTS "next_message_position" integer DEFAULT 1 NOT NULL; |
There was a problem hiding this comment.
🔍 Existing chats lose their transcript when the message store moves to chat_messages
chats.messages is retired in place (declared but never read or written) and the transcript now lives in chat_messages, which migration 0002 creates empty. Any conversation already persisted in a deployed environment therefore reads back as an empty transcript: getChatMessages selects only from chat_messages, and falls back to [] (not null) once chatExists confirms the chat. chats.next_message_position also starts at 1 for those rows, so new messages begin at position 1 with no back-fill. This is presumably acceptable while the feature is flag-gated, but it is a silent history loss for anyone who has already used the agent — worth confirming no internal environment has conversations worth keeping, or adding a one-time back-fill.
Was this helpful? React with 👍 or 👎 to provide feedback.
| // An environment JWT may read a queue, the way it already reads that queue's metrics. | ||
| // Same data class — name, depth, limit, paused — and the `queues` scope still gates it. | ||
| allowJWT: true, |
There was a problem hiding this comment.
🔍 Enabling allowJWT on queue retrieve does not actually reach the agent's minted JWT
The new test file apps/webapp/test/queueRetrieveJwt.test.ts justifies allowJWT: true by saying the agent reads a queue's live row through its exchanged environment JWT and previously got a 401 that the model read as "the queue does not exist". But the exchange in internal-packages/dashboard-agent/src/tool-api-client.ts requests only ["read:runs", "read:deployments", "read:errors", "read:query"], and this route still gates on read for { type: "queues" }. No tool in tool-api.ts calls the retrieve route either — get_queue hits /api/v1/queues/:q/metrics, which is gated on query:queue_metrics and is covered by read:query. So the change is currently inert for the agent; if the intent is for the agent to read the live queue row, read:queues needs adding to the exchanged scope list (and to the delegated token's cap).
Was this helpful? React with 👍 or 👎 to provide feedback.
The system behind the dashboard agent: everything except the UI, which follows in #4529
The agent reads runs, errors, queues, deploys and health through the public API with a delegated, read-only user token. It has no database access of its own beyond its conversation store.
Structure
Each layer only knows the one above it. The UI PR sits on top and knows all four.
System-level changes
userActorEnvironment.server.ts) enforces it; routes call it rather than deriving the rule.img-srcdrops its wildcard.Notes
canAccessDashboardAgent; no behavior change with the flag off.@trigger.dev/core(report schemas) — see the changeset.